Skip to content

Test infra follow-ups #2: library invariants, helpers.py coverage 45→86%, cov in CI#146

Merged
JerBouma merged 1 commit into
JerBouma:mainfrom
dokson:feature/test-followups-2
May 19, 2026
Merged

Test infra follow-ups #2: library invariants, helpers.py coverage 45→86%, cov in CI#146
JerBouma merged 1 commit into
JerBouma:mainfrom
dokson:feature/test-followups-2

Conversation

@dokson
Copy link
Copy Markdown
Contributor

@dokson dokson commented May 19, 2026

Summary

Closes the test-infra follow-ups deferred in #140. Five logically-distinct improvements bundled because they all touch tests/ together and share the same audit pass (pytest --cov=financedatabase --cov-report=term-missing).

Coverage 78% → 92%. Tests 32 → 49. No snapshot file modified — only test code, config, and the workflow step.

Improvement Files
1. test_exchange_market_one_to_one uses the library tests/test_equities.py
2. Cover the helpers.py 45% → 86% gap (+ asset-class invalid-value tests) tests/test_*.py
3. Behavioural smoke asserts in every test_select all 7 asset-class test files
4. Coverage reporting in CI .github/workflows/testing.yml
5. Deferred items called out explicitly (xdist, parametrize)

1. test_exchange_market_one_to_one now uses the library

After #140, the library reads local data (use_local_location=True) and the conftest regenerates compression artifacts from database/*.csv at import time, so equities.select() is in sync with the checked-out source of truth for the test session. The previous pd.read_csv("database/equities.csv") workaround is no longer needed:

-    import pandas as pd
-    from pathlib import Path
-    csv_path = Path(__file__).resolve().parents[1] / "database" / "equities.csv"
-    df = pd.read_csv(csv_path, dtype=str)
+    df = equities.select()

The invariant is identical (each exchange code maps to exactly one market label); only the data source is now the library, consistent with every other test in the file.

2. Coverage of helpers.py and asset-class validation paths

Targeted tests for every uncovered branch identified by pytest --cov. All pure asserts (no recorder), following the test_sec_enrichment_controller.py precedent. New tests added in tests/test_equities.py:

New test Branch covered (helpers.py / Equities.py)
test_search_with_list_of_index search(index=[…]) list-membership
test_search_case_sensitive search(case_sensitive=True)
test_search_case_sensitive_with_list list + case-sensitive combo (helpers.py:124-141)
test_search_invalid_column_is_ignored key not in data_filter.columns warning
test_to_toolkit_raises_without_financetoolkit to_toolkit ImportError path (mocked)
test_to_toolkit_success_path to_toolkit success path (mocked Toolkit, helpers.py:236-265)
test_init_raises_on_request_failure __init__ RequestException path (mocked)
test_module_show_options_raises_on_invalid_selection module-level show_options ValueError paths
test_module_show_options_raises_on_request_failure module-level show_options network failure (mocked)
test_module_show_options_local_location module-level show_options(use_local_location=True) (helpers.py:320, 336-341)

Plus one parallel test per asset class in tests/test_<asset>.py:

New test Branch covered
test_select_with_invalid_value_raises select(<filter>=...) ValueError for unknown values — covers the validate-and-raise blocks unique to each asset class (e.g. ETFs.py:83, 96, 109, 115-126)

The asset-class test exercises every filter column declared on that class's select() signature, so adding a new filter to the API naturally requires the test to be updated.

3. Behavioural smoke assert in every asset-class test_select

Snapshot tests pass even when the library returns an empty DataFrame (the snapshot, regenerated under the same broken state, would just be empty too — and the test would pass silently). One cheap assertion per file catches that class of regression:

smoke = equities.select()
assert not smoke.empty
assert "country" in smoke.columns

Applied to all 7 asset-class test files. Trivial runtime cost; meaningful safety net against the empty-DataFrame failure mode that pure snapshot tests can't detect.

4. Coverage reporting in CI

pytest-cov was already in [tool.poetry.group.dev.dependencies] but wasn't invoked by the workflow. Wired it in:

-                    poetry run pytest tests/
+                    poetry run pytest tests/ --cov=financedatabase --cov-report=term-missing

Coverage now appears in the CI log; gaps become visible at PR review time without anyone having to run pytest locally. No Codecov upload — the terminal report is enough for review-time signal; uploading to a third party would add a token + service dependency and can be a focused follow-up if you want a coverage badge / trend chart.

Coverage breakdown (before → after)

File Before this PR After this PR
helpers.py 45% 86%
ETFs.py 73% 90%
Equities.py 88% 94%
Funds.py 83% 92%
Indices.py 89% 93%
Cryptos.py / Currencies.py / Moneymarkets.py 90% 97%
Total 78% 92%

5. What this PR explicitly does NOT do (still deferred)

  • pytest-xdist parallel execution. Tried it (-n auto); the conftest's compression-regen at import time runs once per worker, racing on the same files. For a ~25s suite the per-worker import overhead actually slows the wall time. The fix is non-trivial (coordinate regen across workers via a lock file, or convert the regen to a pytest-xdist-compatible session fixture) and not worth doing for a suite this small.
  • @pytest.mark.parametrize / class-based grouping. Would change pytest node IDs and break the snapshot positional naming convention (test_select.csv, test_select_1.csv, …, test_select_12.csv). Would require renaming 90+ snapshot files in one go — preferred as a focused refactor PR if you want it.
  • Replacing the vendored Record/PathTemplate/Recorder (~220 LOC of test infra in conftest.py) with syrupy. Mentioned in passing because it would delete a lot of code, but again touches every snapshot file. Out of scope here.

Test plan

…py coverage + cov in CI

Closes the test-infra follow-ups deferred in JerBouma#140. All four items below
land in this single PR. Coverage 78% -> 92%, tests 32 -> 49.

## 1. `test_exchange_market_one_to_one` now uses the library

After JerBouma#140, `tests/test_equities.py` instantiates
`fd.Equities(use_local_location=True)` and the conftest regenerates
compression from CSV at import time. The library output is therefore
in sync with `database/equities.csv` for the test session, so the
consistency invariant can be expressed in the same idiom as the
surrounding tests:

```diff
-    import pandas as pd
-    from pathlib import Path
-    csv_path = Path(__file__).resolve().parents[1] / "database" / "equities.csv"
-    df = pd.read_csv(csv_path, dtype=str)
+    df = equities.select()
```

## 2. helpers.py / asset-class coverage gap (was 45% on helpers.py -> 86%)

Added small targeted tests for the previously-uncovered branches found
by `pytest --cov=financedatabase --cov-report=term-missing`. All pure
asserts (no recorder), following the `test_sec_enrichment_controller.py`
precedent.

In `tests/test_equities.py`:

| New test | Branch covered |
|---|---|
| `test_search_with_list_of_index` | `search(index=[…])` list-membership |
| `test_search_case_sensitive` | `search(case_sensitive=True)` |
| `test_search_case_sensitive_with_list` | list + case-sensitive combo (helpers.py:124-141) |
| `test_search_invalid_column_is_ignored` | `key not in data_filter.columns` warning |
| `test_to_toolkit_raises_without_financetoolkit` | `to_toolkit` ImportError path (mocked) |
| `test_to_toolkit_success_path` | `to_toolkit` success path (mocked Toolkit, helpers.py:236-265) |
| `test_init_raises_on_request_failure` | `__init__` `RequestException` path (mocked) |
| `test_module_show_options_raises_on_invalid_selection` | module-level `show_options` `ValueError` paths |
| `test_module_show_options_raises_on_request_failure` | module-level `show_options` network failure (mocked) |
| `test_module_show_options_local_location` | module-level `show_options(use_local_location=True)` (helpers.py:320, 336-341) |

In every asset-class `tests/test_<asset>.py` (cryptos, currencies,
equities, etfs, funds, indices, moneymarkets):

| New test | Branch covered |
|---|---|
| `test_select_with_invalid_value_raises` | `select(<filter>=...)` ValueError for unknown values — covers the validate-and-raise blocks unique to each asset class (e.g. `ETFs.py:83, 96, 109, 115-126`) |

The asset-class test exercises every filter column declared on that
class's `select()` signature, so adding a new filter automatically
needs a value validation when the test is updated.

## 3. Behavioural smoke assert in every asset-class `test_select`

Snapshot tests pass even when the library returns an empty DataFrame
(the snapshot, regenerated under the same broken state, would just be
empty too — and the test would pass silently). One cheap assertion per
file catches that class of regression:

```python
smoke = equities.select()
assert not smoke.empty
assert "country" in smoke.columns
```

Applied to all 7 asset-class test files.

## 4. Coverage reporting in CI

`pytest-cov` was already in `[tool.poetry.group.dev.dependencies]` but
wasn't invoked by the workflow. Wired it in:

```diff
-                    poetry run pytest tests/
+                    poetry run pytest tests/ --cov=financedatabase --cov-report=term-missing
```

Coverage now appears in the CI log; gaps become visible without local
runs. No Codecov upload — terminal report is enough for review-time
signal; uploading to a third party would add a token + service
dependency and can be a focused follow-up.

## Coverage breakdown (before -> after)

| File | Before | After |
|---|---:|---:|
| `helpers.py` | 45% | **86%** |
| `ETFs.py` | 73% | 90% |
| `Equities.py` | 88% | 94% |
| `Funds.py` | 83% | 92% |
| `Indices.py` | 89% | 93% |
| `Cryptos.py` / `Currencies.py` / `Moneymarkets.py` | 90% | 97% |
| **Total** | **78%** | **92%** |

## What this PR deliberately does NOT do (still deferred)

- `pytest-xdist` parallel execution. Tried it (`-n auto`); the
  conftest's compression-regen at import time runs once per worker,
  racing on the same files. For a ~25s suite the per-worker import
  overhead actually slows the wall time. The fix is non-trivial
  (coordinate regen across workers via a lock file) and not worth doing
  for a suite this small.
- `@pytest.mark.parametrize` / class-based grouping — would change
  pytest node IDs and break the snapshot positional naming convention.

## Test plan

- [ ] `pytest tests/` — 49 tests pass (was 32 before this PR)
- [ ] CI coverage line appears in the run log
- [ ] `black --check tests/` clean
- [ ] No snapshot files modified — only test code, config, and the
      `.github/workflows/testing.yml` step
@JerBouma JerBouma merged commit 59c8b1c into JerBouma:main May 19, 2026
3 checks passed
@dokson dokson deleted the feature/test-followups-2 branch May 19, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants